feat: finaliza o layout do calendario usando BEM e SCSS#5442
feat: finaliza o layout do calendario usando BEM e SCSS#5442luis-botelho wants to merge 2 commits into
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still critical functional issues that prevent the task from fully meeting the requirements. First, the day numbers do not reliably appear as required: in src/styles/index.scss the @for loop is inside .calendar__day, so &:nth-child(#{$i}) tries to match a .calendar__day that is the nth-child of another .calendar__day, which never happens, so the ::before content with numbers is never shown; this breaks the requirement to number days via ::before and @for. To fix this, move the loop to the .calendar scope and target .calendar__day:nth-child(#{$i})::before, so the nth-child is calculated relative to .calendar, which is the parent of the day elements. Second, the task explicitly requires styles to be written in src/styles/main.scss, but you currently keep everything in src/styles/index.scss and link styles/index.scss from index.html, which violates the specified entry file requirement; you should move/rename index.scss to main.scss and update the <link> href accordingly. Additionally, src/styles/__calendar.scss contains a duplicate and incorrect calendar implementation where the @for loop is attached to .calendar itself (&:nth-child(#{$i})::before inside .calendar), which is both non-functional and confusing; either remove this file or correct it and reuse it consistently so there is only one clear, working implementation. Aside from these issues, your flex layout, modifiers, hover effects, and use of SCSS features are well done, so once you fix the numbering logic and the main entry file, your solution will align well with the task requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> |
There was a problem hiding this comment.
This @for loop is attached to .calendar (&:nth-child(#{$i})::before inside .calendar), so it targets .calendar itself, not each .calendar__day. It means numbers won’t be applied to individual days from this file and duplicates the correct implementation you already have inside .calendar__day in index.scss. This violates the requirement that numbers be added to each day via ::before and @for—consider moving this loop inside &__day or remove this duplication.
|
|
||
| &__day { | ||
| display: flex; | ||
| align-items: center; |
There was a problem hiding this comment.
The task requires: "Write styles in src/styles/main.scss instead of src/style.css". You currently link to styles/index.scss and keep all styles there, so this violates the description’s requirement about the styles entry file and needs to be moved to main.scss with the link updated accordingly.
| @for $i from 1 through 31 { | ||
| // O :nth-child encontra a div correta. O ::before injeta o texto nela. | ||
| // A sintaxe #{$i} é como o ${i} das template strings do JavaScript. | ||
| &:nth-child(#{$i})::before { | ||
| content: '#{$i}'; | ||
| } | ||
| } |
There was a problem hiding this comment.
This file contains an alternative .calendar implementation where the @for loop is applied at the .calendar level, so &:nth-child(#{$i})::before targets .calendar rather than .calendar__day. Since index.scss already contains the correct implementation, this extra, inconsistent version is confusing and should either be removed or corrected and reused consistently.
| $days: mon, tue, wed, thu, fri, sat, sun; | ||
|
|
||
| * { | ||
| box-sizing: border-box; |
There was a problem hiding this comment.
The task description requires styles to be written in src/styles/main.scss and linked from HTML. Here you are linking styles/index.scss instead, which conflicts with the requirement to use main.scss as the SCSS entry file.
| @for $i from 1 through 31 { | ||
| &:nth-child(#{$i})::before { | ||
| content: '#{$i}'; | ||
| } | ||
| } |
There was a problem hiding this comment.
The @for loop that assigns numbers should target each .calendar__day based on its position inside .calendar, but placing it inside &__day makes & be .calendar__day, so &:nth-child(#{$i}) refers to a .calendar__day that is the nth-child of another .calendar__day, which never matches. Move this loop to the .calendar scope and target .calendar__day:nth-child(#{$i})::before so numbers actually appear.
|
|
||
| .calendar { | ||
| display: flex; | ||
| flex-wrap: wrap; |
There was a problem hiding this comment.
This separate __calendar.scss duplicates calendar styles and also contains a broken numbering rule: &:nth-child(#{$i})::before here resolves to .calendar:nth-child(...)::before, so it will never hit the day cells. In addition, this file is not imported from your main SCSS entry, so its rules are unused. Consider either removing it or correctly integrating and fixing the selector to target .calendar__day:nth-child(...) instead.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because it fully meets the functional and structural requirements of the calendar task. The calendar is correctly centered, uses flex with a computed width for exactly seven columns, and has 31 .calendar__day elements without any hardcoded numbers in the HTML. Day cells are styled as 100px grey squares with borders, and the numbers are generated via an @for loop using ::before and nth-child, which aligns with the SCSS requirements. The calendar--start-day-* and calendar--month-length-* modifiers are implemented using @each and @for respectively, and changing these classes in the HTML correctly shifts the starting day and hides extra days. Hover behavior (pointer, pink background #FFBFCB, 20px upward movement, 0.5s transition) matches the specification. BEM naming is clean (calendar, calendar__day, calendar--...), and styles are properly placed in src/styles/main.scss and linked from index.html. Nicely done implementing all checklist items without introducing any critical or blocking issues.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
[ x] Changing 'month-lengh' and 'start-day' modifier in the code element
reflects in changing calendar layout
Typical MistakesfromBEMlesson theory are checked.